Repository navigation
fix(@angular/build): count statically imported chunks in the initial total - #33793
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the chunk optimizer in the Angular build package to register the newly created file record in the 'original.initialFiles' map before pushing it to the analysis queue. I have no feedback to provide as there are no review comments.
…total `optimizeChunks` removes every used chunk from `initialFiles` and then re-walks the optimized graph to restore the ones the main entry still imports statically. The walk builds an `InitialFileRecord` for each import but never stores it, so the entries are analyzed and then dropped. Because the records were deleted just above, `existingRecord` is always undefined, and the map ends up holding only the configured entry points. Chunks the browser must fetch before the application can run are therefore excluded from "Initial total" and reported under "Lazy chunk files" instead. On a large application this understated the initial payload by roughly 3x: 377.35 kB reported against 1.35 MB actually required, with a 919 kB statically imported chunk listed as lazy. `BundlerContext#bundle` performs the equivalent walk and does call `initialFiles.set` before pushing the entry; this aligns the optimizer with it. Fixes angular#33773
c43e69e to
1b59423
Compare
|
This pull request has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #33773
Problem
optimizeChunksdrops every used chunk frominitialFiles, keeping only the main entry:It then re-walks the optimized graph to restore the chunks the entry still imports statically — but the record it builds is never written back to the map:
Since those entries were just deleted,
existingRecordis alwaysundefined, so every chunk is analyzed and then discarded.initialFilesends up containing only the configured entry points, which is exactly what "Initial total" sums — so chunks the browser must download before the app can run are excluded from the total and reported under "Lazy chunk files".BundlerContext#bundleperforms the equivalent walk intools/esbuild/bundler-context.tsand does store the record before pushing:This change aligns the optimizer with that behaviour.
Note the esbuild metafile is correct —
--stats-jsonreports these askind: "import-statement"frommain. Only the post-optimization bookkeeping is wrong.Effect
Same application, production build, only this line changed.
Before
After
Independently cross-checked by walking
index.htmlthrough staticimport/fromedges only (skippingimport()): 1172.0 kB of JS plus 149.3 kB of CSS, matching the corrected total.Beyond the reported number, this also feeds
generateIndexHtmland the server manifest's initial-files set, so those consume the same corrected map.On test coverage
I was not able to build a fixture that exercises this path, and I would appreciate guidance.
The failure is only observable when the optimizer's output still contains
main -> chunkstatic edges. In small applications Rolldown hoists all entry-reachable shared code into the main chunk, so no such chunk survives to be lost — whichchunk-optimization-server_spec.tsalready relies on, assertinghasFileMatch('dist/browser', /^chunk-/)is false as proof the pass ran.I tried a fresh
ng newon 22.1.3 across several shapes — 1 and 8 lazy routes, a shared component, a ~2 MB shared module producing a 1.72 MBmain, and eight ~154 kB lazy routes sharing vendor code — and every one produced zero static chunks frommain, leaving nothing to misreport. The affected application has ~114 lazy chunks.If a fixture shape that retains a shared chunk would be acceptable, or if you would prefer a unit test asserting the invariant directly (every
importsPerFile[mainFile]entry present ininitialFilesafteroptimizeChunks), I am happy to add it.